Leakage modeling: bugfixes for wildcard error and gauge optimization #671
Conversation
…obust to different labelings of the idenity gate, and make the returned model`s default_gauge_group have a direct sum structure; make the default leakage gaugeopt suite 2-stage, where the second stage is over the default gauge group in the target model)
rileyjmurray
requested review from
coreyostrove and
sserita
and removed request for
a team
…Have lagoified_gopparams_dicts apply the direct sum group by default, since apparently the target model stored in list-of-dicts representation of `stdgaugeopt` suite overrides my attempt at setting the default gauge group
…pecific gate causes leakage
…orrectness test for leakage GST.
|
@coreyostrove I believe this is ready for review. When this guy is merged I can get to work on finalizing #669, which targets develop. Once that guy is merged I'll do the expansion of general leakage modeling functionality. That PR will include an overhaul of some functions in gaugeopt.py to avoid awkward use of ExplicitOpModelCalc. |
There was a problem hiding this comment.
Thanks for all of your hard work, @rileyjmurray! I've left a few comments and questions for you which should hopefully be fairly quick to address.
| if transform is not None: | ||
| self_mx = self_mx @ transform | ||
| if isinstance(inv_transform, _mt.IdentityOperator): | ||
| other_mx = other_mx @ transform |
There was a problem hiding this comment.
I don't understand lines 423 and 424, what do these do?
| vec = transform.T @ vec | ||
| return _ot.frobeniusdist_squared(vec, other_spam_vec.to_dense()) | ||
| if isinstance(inv_transform, _mt.IdentityOperator): | ||
| other_vec = transform.T @ other_vec |
| if isinstance(basis, str): | ||
| ss = _ExplicitStateSpace(['all'], [udim]) | ||
| self.basis = _BuiltinBasis("std", ss) | ||
| basis = _BuiltinBasis("std", ss) |
There was a problem hiding this comment.
This looks like it always sets the basis to the standard basis when a string input is given. Shouldn't this be using the value of the basis kwarg to set this value?
|
|
||
|
|
||
| DEFAULT_MIN_PROB_CLIP = 1e-4 | ||
| DEFAULT_RADIUS = 1e-4 |
There was a problem hiding this comment.
Should these be class attributes of the base ObjectiveFunction class instead of at the module level? (I don't feel strongly either way.)
|
|
||
| # Whether this rank is the "leader" of all the processors accessing the same shared self.jac and self.probs mem. | ||
| # Only leader processors should modify the contents of the shared memory, so we only apply operations *once* | ||
| # `unit_ralloc` is the group of all the procs targeting same destination into self.obj |
There was a problem hiding this comment.
Possible git merge issue? The comments and code being removed in terms and lsvec were recently added in PR #660. In any case these changes should be undone.
|
|
||
|
|
||
| def _compute_1d_reference_values_and_name(estimate, badfit_options, gaugeopt_suite=None): | ||
| def _compute_1d_reference_values_and_name(target_model, gopped_models, gaugeopt_suite): |
There was a problem hiding this comment.
If we are going to re-specialize this function for diamond distance (and retreat on extensions to other reference values for the time being) then this change should propagate to the wildcard1d_reference attribute of GSTBadFitOptions. I.e. we shouldn't have this attribute, as then a user may improperly believe it does something.
| s = lagoified_gopparams_dicts(gopparams_dicts) | ||
| t = lagoified_gopparams_dicts(gopparams_dicts) | ||
| t[-1]['gates_metric'] = 'fidelity' | ||
| t[-1]['spam_metric'] = 'fidelity' |
There was a problem hiding this comment.
The docstring made me think your new gaugeopt suite would be using fidelity as the default, but you're using 'frobenius squared' as far as I can tell.
2 participants
This PR is split off from #664. It fixes how wildcard error is computed in leakage modeling. When merged it will resolve #652. It also includes changes to the default leakage-aware gauge optimization suite. The latter changes are needed to prevent gauge optimization from trying to spurriously "spread out" relational errors across available gates.
This PR adds a meaningful correctness test for leakage GST modeling. To reduce the time of the test I used a huge number of shots per circuit in the simulated dataset. This led to errors with "min_prob_clip" constants in objective function evaluation. In order to resolve those errors I replaced hard-coded instances of that constant's default value (1e-4) to values of a module-wide constant; I made similar changes for "radius" constants in objective functions.
(This PR is a superset of the since-closed PR #670, which started from develop instead of bugfix.)